-
Notifications
You must be signed in to change notification settings - Fork 164
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Script to check connectivity to azure #2734
Conversation
4e02d1b
to
5341977
Compare
pkg/debug/scripts/azure-check.sh
Outdated
__EOT__ | ||
} | ||
|
||
for prog in base64 openssl xxd curl |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would be nice to wrap the code in main()
function
pkg/debug/scripts/azure-check.sh
Outdated
done | ||
|
||
# the same as in zedUpload | ||
authorization="SharedKey" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there a reason why some variables are in CAPS_LOCK, and other are not? Maybe keep all of them in lower case?
pkg/debug/scripts/azure-check.sh
Outdated
# Build the signature string | ||
|
||
if [ -z "$LIMIT_IN_BYTES" ] || [ -z "$BLOB_NAME" ]; then | ||
canonicalized_headers="${x_ms_date_h}\n${x_ms_version_h}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd suggest to use braces everywhere for consistency
pkg/debug/scripts/azure-check.sh
Outdated
|
||
# in case of port provided we cannot use subdomain | ||
# checked with azurite | ||
if echo "$DOMAIN_NAME" | grep -q ":"; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's generally better to avoid starting sub process (grep
). Something like this reads less obvious (thanks to /bin/sh
instead of /bin/bash
), but better from execution point of view:
case "${DOMAIN_NAME}" in
*:*) echo "Found a colon";;
* ) echo "not found";;
esac
pkg/debug/scripts/azure-check.sh
Outdated
|
||
# Create the HMAC signature for the Authorization header | ||
# shellcheck disable=SC2059 | ||
signature=$(printf "$string_to_sign" | openssl dgst -sha256 -mac HMAC -macopt "hexkey:$decoded_hex_key" -binary | base64) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
might be worth to check if openssl
returned an error
Azure use signature checking across provided headers. It would be nice to have a tool to debug unexpected intermediate headers modifications that may lead to errors like "The MAC signature found in the HTTP request is not the same as any computed signature". Even without access_key the script allow to show headers server use to calculate the signature. Signed-off-by: Petr Fedchenkov <giggsoff@gmail.com>
5341977
to
e4f8dee
Compare
@zededa-yuri thanks for recommendations, modified. |
@giggsoff @eriknordmark Let's merge this? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's run the tests.
Azure use signature checking across provided headers. It would be
nice to have a tool to debug unexpected intermediate headers
modifications that may lead to errors like "The MAC signature found in
the HTTP request is not the same as any computed signature". Even
without access_key the script allow to show headers server use to
calculate the signature.
Signed-off-by: Petr Fedchenkov giggsoff@gmail.com